Files that changed from the base of the PR and between 4837c7d and 3e3afe7.

• packages/sdk/src/api/converter.ts (9 hunks)
• packages/sdk/src/api/yorkie/v1/resources.proto (2 hunks)
• packages/sdk/src/api/yorkie/v1/resources_pb.ts (1 hunks)
• packages/sdk/src/api/yorkie/v1/yorkie_connect.ts (2 hunks)
• packages/sdk/src/api/yorkie/v1/yorkie_pb.ts (1 hunks)
• packages/sdk/src/document/change/change_id.ts (5 hunks)
• packages/sdk/src/document/change/change_pack.ts (4 hunks)
• packages/sdk/src/document/crdt/root.ts (2 hunks)
• packages/sdk/src/document/document.ts (13 hunks)
• packages/sdk/src/document/time/version_vector.ts (1 hunks)
• packages/sdk/test/helper/helper.ts (4 hunks)
• packages/sdk/test/integration/array_test.ts (2 hunks)
• packages/sdk/test/integration/gc_test.ts (5 hunks)
• packages/sdk/test/unit/document/crdt/root_test.ts (3 hunks)
• packages/sdk/test/unit/document/document_test.ts (3 hunks)
• packages/sdk/test/unit/document/gc_test.ts (15 hunks)

• packages/sdk/src/api/yorkie/v1/resources_pb.ts
• packages/sdk/src/api/yorkie/v1/yorkie_pb.ts

packages/sdk/src/api/yorkie/v1/yorkie_connect.ts (2)

16-16: Tool version updated to v1.4.0.

The update to protoc-gen-connect-es from v1.2.0 to v1.4.0 is noted. This change is likely to bring improvements and new features from the tool that could benefit the TypeScript generation process.

---

105-105: Immutable export of YorkieService enhances type safety.

The use of as const in the export statement of YorkieService ensures that its properties are treated as immutable, which enhances type safety and consistency in its usage. This change is beneficial for maintaining the integrity of the service's structure across different parts of the application.

packages/sdk/src/document/time/version_vector.ts (1)

26-147: Introduction of VersionVector class supports enhanced synchronization.

The new VersionVector class provides essential functionality for managing version vectors, which are crucial for tracking changes and handling document synchronization across clients. This class supports operations such as setting and retrieving lamport timestamps, computing maximum values, and other vector-related operations, aligning well with the PR's objectives to enhance synchronization capabilities.

packages/sdk/src/document/change/change_pack.ts (1)

Line range hint 55-198: Enhancements to ChangePack class improve version management.

The modifications to the ChangePack class, including the addition of new properties (snapshotVersionVector, minSyncedVersionVector, versionVector) and related methods (getVersionVector, getMinSyncedVersionVector, getSnapshotVersionVector), significantly enhance the class's capability to manage versioning information. These changes are crucial for scenarios involving collaborative document editing and synchronization, aligning well with the PR's objectives to enhance document synchronization capabilities.

packages/sdk/test/unit/document/crdt/root_test.ts (2)

16-16: Approved import statement for MaxVersionVector.

The addition of MaxVersionVector is necessary for the updated garbage collection logic that utilizes version vectors.

---

107-107: Approved changes to garbage collection tests.

The modification to use MaxVersionVector([]) instead of MaxTimeTicket aligns with the PR's objective of enhancing synchronization capabilities. Ensure that the new garbage collection logic is thoroughly tested to confirm its effectiveness.

Run the following script to verify the new garbage collection logic:

Also applies to: 133-133

Verification successful

---

Verification successful: New garbage collection logic is consistently tested.

The garbageCollect function is used with MaxVersionVector across various test files, confirming that the new logic is being thoroughly tested. The presence of MaxTimeTicket in benchmark tests appears to be intentional for performance comparisons.

• Test files using MaxVersionVector:
  • packages/sdk/test/integration/gc_test.ts
  • packages/sdk/test/unit/document/gc_test.ts
  • packages/sdk/test/unit/document/crdt/root_test.ts

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the new garbage collection logic.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type typescript -A 5 $'garbageCollect'

Length of output: 77

---

Script:

#!/bin/bash
# Description: Verify the new garbage collection logic by searching for the function usage without specifying a file type.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg -A 5 $'garbageCollect'

Length of output: 15312

packages/sdk/src/document/change/change_id.ts (5)

86-99: Approved changes to the syncClocks method of ChangeID.

The update to incorporate the version vector in the syncClocks method ensures better synchronization of logical clocks, aligning with the PR's objectives. Verify the integration of this method with other components to ensure compatibility.

Run the following script to verify the integration of the syncClocks method:

Verification successful

---

Integration of the syncClocks method verified.

The syncClocks method is integrated into the document handling logic, ensuring synchronization of logical clocks during change processing. This aligns with the intended functionality. Further manual testing is recommended to ensure comprehensive integration.

• Location of usage: packages/sdk/src/document/document.ts

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration of the `syncClocks` method.

# Test: Search for the method usage. Expect: Only occurrences of the new implementation.
rg --type typescript -A 5 $'syncClocks'

Length of output: 73

---

Script:

#!/bin/bash
# Description: Verify the integration of the `syncClocks` method.

# Test: Search for the method usage. Expect: Only occurrences of the new implementation.
rg --type ts -A 5 $'syncClocks'

Length of output: 1175

---

72-80: Approved changes to the next method of ChangeID.

The update to use the version vector in the next method enhances the tracking of changes, aligning with the PR's objectives. Verify the functionality of this method to ensure it correctly handles the version vector.

Run the following script to verify the functionality of the next method:

Verification successful

---

Verified usage of the next method in the codebase.

The next method of the ChangeID class is utilized in packages/sdk/src/document/document.ts, confirming its integration into the document change handling process. This aligns with the intended functionality of the method to manage version vectors effectively.

• File: packages/sdk/src/document/document.ts

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the functionality of the `next` method.

# Test: Search for the method usage. Expect: Only occurrences of the new implementation.
rg --type typescript -A 5 $'next'

Length of output: 67

---

Script:

#!/bin/bash
# Description: Verify the functionality of the `next` method.

# Test: Search for the method usage. Expect: Only occurrences of the new implementation.
rg --type ts -A 5 $'next'

Length of output: 44993

---

188-192: Approved addition of the getVersionVector method to ChangeID.

The new getVersionVector method provides external access to the version vector, enhancing the tracking and management of changes, aligning with the PR's objectives. Verify the functionality of this method to ensure it correctly retrieves the version vector.

Run the following script to verify the functionality of the getVersionVector method:

Verification successful

---

The getVersionVector method is well-integrated and tested.

The getVersionVector method is used in multiple test files and other parts of the SDK, indicating that its functionality is being verified and utilized effectively. No further action is needed to verify its functionality.

• Integration Tests: Found in packages/sdk/test/integration/gc_test.ts.
• Unit Tests: Found in packages/sdk/test/unit/document/gc_test.ts.
• SDK Usage: Found in packages/sdk/src/document/document.ts and packages/sdk/src/api/converter.ts.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the functionality of the `getVersionVector` method.

# Test: Search for the method usage. Expect: Only occurrences of the new implementation.
rg --type typescript -A 5 $'getVersionVector'

Length of output: 79

---

Script:

#!/bin/bash
# Description: Verify the functionality of the `getVersionVector` method.

# Test: Search for the method usage in TypeScript files.
rg --type ts -A 5 'getVersionVector'

Length of output: 6339

---

106-113: Approved addition of the setClocks method to ChangeID.

The new setClocks method enhances the flexibility of the ChangeID class by allowing external adjustments to the clocks, aligning with the PR's objectives. Verify the functionality of this method to ensure it correctly handles the version vector and lamport timestamp.

Run the following script to verify the functionality of the setClocks method:

---

127-133: Approved changes to the setActor method of ChangeID.

The update to maintain the version vector when changing the actor in the setActor method ensures the integrity of the change ID, aligning with the PR's objectives. Verify the integration of this method with other components to ensure compatibility.

Run the following script to verify the integration of the setActor method:

Verification successful

---

Integration of the setActor method verified.

The setActor method in change_id.ts is effectively integrated with other components, such as in document.ts, where it updates the changeID. This confirms its role in maintaining consistency in versioning. Ensure that related components are tested for compatibility with this update.

• packages/sdk/src/document/change/change_id.ts: setActor method updates changeID with the new actor while maintaining the version vector.
• packages/sdk/src/document/document.ts: Utilizes setActor to manage document changes.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration of the `setActor` method.

# Test: Search for the method usage. Expect: Only occurrences of the new implementation.
rg --type typescript -A 5 $'setActor'

Length of output: 71

---

Script:

#!/bin/bash
# Description: Verify the integration of the `setActor` method.

# Test: Search for the method usage in TypeScript files. Expect: Only occurrences of the new implementation.
rg --glob '*.ts' -A 5 $'setActor'

Length of output: 5340

packages/sdk/test/helper/helper.ts (2)

305-326: Approved addition of the versionVectorHelper function.

The new versionVectorHelper function enhances the logic related to version control and synchronization mechanisms by validating the integrity of version vectors. Verify the functionality of this function to ensure it correctly validates version vectors.

Run the following script to verify the functionality of the versionVectorHelper function:

Verification successful

---

Functionality of versionVectorHelper is verified through integration tests.

The versionVectorHelper function is actively used in the gc_test.ts file, where it is part of assertions to validate version vectors. This indicates that its functionality is being tested in various scenarios, confirming its correct implementation.

• File: packages/sdk/test/integration/gc_test.ts

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the functionality of the `versionVectorHelper` function.

# Test: Search for the function usage. Expect: Only occurrences of the new implementation.
rg --type typescript -A 5 $'versionVectorHelper'

Length of output: 82

---

Script:

#!/bin/bash
# Description: Verify the functionality of the `versionVectorHelper` function.

# Test: Search for the function usage. Expect: Only occurrences of the new implementation.
rg --type ts -A 5 $'versionVectorHelper'

Length of output: 3661

---

291-303: Approved addition of the MaxVersionVector function.

The new MaxVersionVector function is crucial for creating version vectors with maximum lamport values, enhancing synchronization capabilities. Verify the integration of this function with other components to ensure compatibility.

Run the following script to verify the integration of the MaxVersionVector function:

Verification successful

---

MaxVersionVector function is well-integrated and utilized in the codebase.

The MaxVersionVector function is extensively used in various test files, particularly for garbage collection tests, confirming its integration and functionality within the codebase.

• Files where MaxVersionVector is used:
  • packages/sdk/test/unit/document/gc_test.ts
  • packages/sdk/test/unit/document/document_test.ts
  • packages/sdk/test/unit/document/crdt/root_test.ts
  • packages/sdk/test/integration/gc_test.ts
  • packages/sdk/test/integration/array_test.ts

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration of the `MaxVersionVector` function.

# Test: Search for the function usage. Expect: Only occurrences of the new implementation.
rg --type typescript -A 5 $'MaxVersionVector'

Length of output: 79

---

Script:

#!/bin/bash
# Description: Verify the integration of the `MaxVersionVector` function by searching for its usage across the codebase.

# Test: Search for the function usage without specifying the file type.
rg -A 5 'MaxVersionVector'

Length of output: 13632

packages/sdk/src/document/crdt/root.ts (2)

30-30: Approved import statement for VersionVector.

The import of VersionVector is correctly placed and necessary for the updated garbage collection logic.

---

272-287: Approved changes to garbageCollect method with suggestions for further verification.

The method now uses VersionVector to determine the synchronization state of elements, which aligns with the PR's objectives. However, it's crucial to ensure that this change integrates smoothly with other components that interact with the garbageCollect method.

Run the following script to verify the integration of garbageCollect with other components:

Verification successful

---

Integration of garbageCollect method verified successfully.

The garbageCollect method is being used with the new VersionVector signature across multiple test files and source files, indicating that the integration with other components has been updated and is being tested.

• Test Files: The method is called in various test files, ensuring that its functionality is being verified.
• Source Files: The method is used in the document.ts file, confirming its integration with document handling logic.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration of `garbageCollect` method with other components.

# Test: Search for the method usage. Expect: Only occurrences with the new signature.
rg --type typescript -A 5 $'garbageCollect'

Length of output: 77

---

Script:

#!/bin/bash
# Description: Verify the integration of `garbageCollect` method with other components.

# Test: Search for the method usage. Expect: Only occurrences with the new signature.
rg --type ts -A 5 $'garbageCollect'

Length of output: 15322

packages/sdk/src/api/yorkie/v1/resources.proto (3)

46-51: Approved changes to ChangePack message with caution on deprecation handling.

The addition of VersionVector fields enhances the message's version tracking capabilities. However, ensure that the deprecation of min_synced_ticket is handled carefully to maintain backward compatibility.

---

66-67: Approved addition of version_vector field to ChangeID message.

The new version_vector field is crucial for maintaining the version state of changes, aligning with the PR's objectives to enhance version control.

---

69-70: Approved definition of VersionVector message.

The new VersionVector message is well-defined and essential for the enhanced versioning system introduced in this PR.

packages/sdk/test/unit/document/gc_test.ts (7)

21-21: Approved import statement for MaxVersionVector.

The import of MaxVersionVector is correctly placed and necessary for the updated garbage collection tests.

---

60-63: Approved changes to garbage collection test case.

The test case correctly uses MaxVersionVector to pass parameters to the garbageCollect method, reflecting the updated logic.

---

86-89: Approved changes to garbage collection test case when disabled.

The test case correctly checks the behavior of garbage collection when it is disabled, ensuring that the disableGC option is respected.

---

104-107: Approved changes to garbage collection test case for big array.

The test case correctly uses MaxVersionVector for a large array, ensuring that the garbage collection logic is tested with large data structures.

---

125-128: Approved changes to garbage collection test case for nested elements.

The test case correctly uses MaxVersionVector for nested elements, ensuring that the garbage collection logic is tested with complex data structures.

---

153-153: Approved changes to garbage collection test case for text node.

The test case correctly uses MaxVersionVector for a text node, ensuring that the garbage collection logic is tested with text nodes.

---

188-191: Approved changes to garbage collection test case with already removed text node.

The test case correctly uses MaxVersionVector for an already removed text node, ensuring that the garbage collection logic is tested with scenarios involving already removed elements.

packages/sdk/test/integration/gc_test.ts (2)

7-7: Approved import statement for version vector functionality.

The import of MaxVersionVector and versionVectorHelper is essential for the updated garbage collection logic based on version vectors.

---

52-63: Approved changes to garbage collection logic.

The replacement of MaxTimeTicket with MaxVersionVector in garbage collection assertions is consistent with the PR's objectives. Ensure that this change integrates well with existing systems.

Run the following script to verify the integration with existing systems:

Verification successful

---

Verified integration of MaxVersionVector across the codebase.

The usage of MaxVersionVector is consistent across multiple test files, confirming its successful integration with the garbage collection logic. The changes align with the PR's objectives and appear to be well-tested.

• Files with MaxVersionVector usage:
  • packages/sdk/test/integration/gc_test.ts
  • packages/sdk/test/integration/array_test.ts
  • packages/sdk/test/unit/document/gc_test.ts
  • packages/sdk/test/unit/document/crdt/root_test.ts
  • packages/sdk/test/unit/document/document_test.ts

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration of MaxVersionVector in existing systems.

# Test: Search for the usage of MaxVersionVector in other test files. Expect: Consistent usage across tests.
rg --type typescript -A 5 $'MaxVersionVector'

Length of output: 79

---

Script:

#!/bin/bash
# Description: Verify the integration of MaxVersionVector in existing systems.

# Test: Search for the usage of MaxVersionVector in other TypeScript files. Expect: Consistent usage across tests.
rg --type ts -A 5 $'MaxVersionVector'

Length of output: 13642

packages/sdk/src/api/converter.ts (5)

187-200: Review of toVersionVector function:

The implementation correctly handles null inputs and iterates over the vector entries to convert them to the Protobuf format. The use of BigInt is noted, and it's good to see a TODO comment indicating plans to refine this in the future.

---

843-858: Review of fromVersionVector function:

The implementation correctly handles null inputs and iterates over the Protobuf entries to convert them back to the model format. The use of Long.fromString is appropriate here.

---

1523-1529: Review of versionVectorToHex function:

The implementation effectively leverages existing functionality to convert a VersionVector to a hexadecimal string. The use of toVersionVector and bytesToHex is appropriate and well-integrated.

---

1534-1539: Review of hexToVersionVector function:

The implementation correctly handles the conversion of a hexadecimal string back to a VersionVector using a well-defined sequence of conversions. The use of hexToBytes, fromBinary, and fromVersionVector is appropriate and ensures accurate conversion.

---

1673-1674: Review of modifications to the converter export:

The inclusion of versionVectorToHex and hexToVersionVector in the converter export is appropriate and aligns with the utility's role in the SDK. This change facilitates the use of these functions across the SDK where needed.

packages/sdk/src/document/document.ts (5)

Line range hint 1370-1392: Review of modifications to applySnapshot method:

The updated applySnapshot method correctly integrates the snapshotVector, ensuring that the document's version vector is updated along with the snapshot. This is crucial for maintaining version consistency and is well-implemented.

---

1307-1315: Review of modifications to garbageCollect method:

The updated garbageCollect method appropriately uses a VersionVector to determine which elements to purge, reflecting a shift towards a more robust versioning system. This change is well-suited for managing document state in a distributed environment.

---

2101-2105: Review of getVersionVector method:

The addition of the getVersionVector method is a crucial enhancement, providing necessary access to the document's version vector. This method is essential for operations that require version checks or comparisons.

---

1859-1866: Review of filterVersionVector method:

The filterVersionVector method is a valuable addition, effectively cleaning up the version vector by removing entries from detached clients. This method plays a crucial role in maintaining the integrity and efficiency of the version vector in collaborative environments.

---

308-312: Review of modifications to SnapshotEvent interface:

The inclusion of snapshotVector in the SnapshotEvent interface is an important update that ensures snapshot events carry all necessary version information. This change is well-aligned with the new versioning strategy and enhances the accuracy of event handling.

packages/sdk/test/integration/array_test.ts (2)

10-10: Confirm import path and module functionality.

Ensure that the import path ../helper/helper correctly points to the MaxVersionVector module and that this module provides the expected functionality.

---

725-725: Review the implementation of MaxVersionVector in garbage collection.

Confirm that the MaxVersionVector is correctly instantiated with the actor ID from doc.getChangeID().getActorID() and that it effectively replaces the previous MaxTimeTicket system without introducing logical errors.

packages/sdk/test/unit/document/document_test.ts (3)

18-20: Confirm import path and module functionality.

Ensure that the import path @yorkie-js-sdk/test/helper/helper correctly points to the MaxVersionVector module and that this module provides the expected functionality.

---

1238-1238: Review the implementation of MaxVersionVector in garbage collection.

Confirm that the MaxVersionVector is correctly instantiated with the actor ID from doc.getChangeID().getActorID() and that it effectively replaces the previous MaxTimeTicket system without introducing logical errors.

---

1254-1254: Review the implementation of MaxVersionVector in garbage collection.

Confirm that the MaxVersionVector is correctly instantiated with the actor ID from doc.getChangeID().getActorID() and that it effectively replaces the previous MaxTimeTicket system without introducing logical errors.

Files that changed from the base of the PR and between 3e3afe7 and 6f383e6.

• packages/sdk/test/integration/client_test.ts (1 hunks)
• packages/sdk/test/integration/gc_test.ts (16 hunks)
• packages/sdk/test/integration/tree_test.ts (1 hunks)

• packages/sdk/test/integration/client_test.ts
• packages/sdk/test/integration/tree_test.ts

packages/sdk/test/integration/gc_test.ts (2)

7-7: New imports added for version vector handling.

The addition of MaxVersionVector and versionVectorHelper from '../helper/helper' is crucial for the new version vector-based garbage collection logic. Ensure that these utilities are properly implemented and tested in their respective modules.

---

83-96: Approved enhancements to version tracking in multi-client scenarios.

The detailed use of versionVectorHelper in assertions significantly enhances the precision of version tracking. Consider adding more tests to cover potential edge cases in multi-client environments.

Would you like me to help generate additional test cases for this scenario?

Also applies to: 118-123, 129-135, 139-145, 150-156, 161-167, 172-178, 183-189, 194-200, 205-211, 237-250, 257-262, 268-274, 277-283, 288-294, 299-305, 310-316, 321-327, 333-339, 344-350, 374-387, 395-400, 405-411, 414-420, 427-433, 438-444, 449-455, 460-466, 471-477, 482-488, 520-533, 544-549, 555-561, 564-570, 577-583, 588-594, 605-610, 630-635, 639-644, 648-653, 657-662, 675-682, 687-692, 697-702, 707-712, 734-739, 741-746, 748-753, 756-761, 764-770, 773-779, 783-788, 791-797, 801-807, 811-817, 820-826, 829-829, 847-850, 865-870, 872-878, 884-889, 893-898, 902-907, 911-916, 918-924, 926-932, 940-946, 948-954, 956-962, 978-984, 986-992, 997-1003, 1005-1011, 1034-1047, 1060-1065, 1072-1077, 1084-1089, 1096-1101, 1103-1109, 1111-1117, 1128-1134, 1136-1142, 1144-1150, 1157-1163, 1165-1171, 1176-1182, 1184-1190, 1192-1192, 1220-1245, 1247-1258, 1260-1267, 1269-1276, 1278-1287, 1289-1298, 1300-1302, 1303-1310, 1312-1319, 1321-1323, 1324-1333, 1335-1342, 1344-1357, 1363-1390, 1392-1403, 1405-1412, 1414-1421, 1423-1433, 1435-1442, 1444-1451, 1453-1463, 1465-1475, 1477-1486, 1488-1495, 1497-1507, 1509-1516, 1518-1525, 1527-1528, 1530-1540, 1542-1549, 1551-1559, 1561-1568, 1570-1572, 1573-1580, 1582-1583, 1585-1592, 1594-1595

Also applies to: 118-123, 129-135, 139-145, 150-156, 161-167, 172-178, 183-189, 194-200, 205-211, 237-250, 257-262, 268-274, 277-283, 288-294, 299-305, 310-316, 321-327, 333-339, 344-350, 374-387, 395-400, 405-411, 414-420, 427-433, 438-444, 449-455, 460-466, 471-477, 482-488, 520-533, 544-549, 555-561, 564-570, 577-583, 588-594, 605-610, 630-635, 639-644, 648-653, 657-662, 675-682, 687-692, 697-702, 707-712, 734-739, 741-746, 748-753, 756-761, 764-770, 773-779, 783-788, 791-797, 801-807, 811-817, 820-826, 829-829, 847-850, 865-870, 872-878, 884-889, 893-898, 902-907, 911-916, 918-924, 926-932, 940-946, 948-954, 956-962, 978-984, 986-992, 997-1003, 1005-1011, 1034-1047, 1060-1065, 1072-1077, 1084-1089, 1096-1101, 1103-1109, 1111-1117, 1128-1134, 1136-1142, 1144-1150, 1157-1163, 1165-1171, 1176-1182, 1184-1190, 1192-1192, 1220-1245, 1247-1258, 1260-1267, 1269-1276, 1278-1287, 1289-1298, 1300-1302, 1303-1310, 1312-1319, 1321-1323, 1324-1333, 1335-1342, 1344-1357, 1363-1390, 1392-1403, 1405-1412, 1414-1421, 1423-1433, 1435-1442, 1444-1451, 1453-1463, 1465-1475, 1477-1486, 1488-1495, 1497-1507, 1509-1516, 1518-1525, 1527-1528, 1530-1540, 1542-1549, 1551-1559, 1561-1568, 1570-1572, 1573-1580, 1582-1583, 1585-1592, 1594-1595

Files that changed from the base of the PR and between 6f383e6 and 13f528b.

• packages/sdk/test/bench/document.bench.ts (2 hunks)
• packages/sdk/test/bench/text.bench.ts (3 hunks)
• packages/sdk/test/bench/tree.bench.ts (3 hunks)

packages/sdk/test/bench/tree.bench.ts (3)

4-4: Import statement for MaxVersionVector is correctly added.

This import is necessary for the new version vector-based garbage collection logic.

---

60-63: Garbage collection logic updated to use MaxVersionVector.

The update from MaxTimeTicket to MaxVersionVector is correctly implemented. This change should theoretically improve the precision of garbage collection by using version vectors.

Consider running performance benchmarks to ensure that the new garbage collection logic does not adversely affect performance.

---

90-93: Consistent update in garbage collection logic for benchmarkTreeEditGC.

The changes are consistent with those in benchmarkTreeSplitGC, correctly using MaxVersionVector for garbage collection. This should enhance the accuracy of garbage collection based on version vectors.

Ensure consistency in performance with the previous implementation by running benchmarks.

packages/sdk/test/bench/document.bench.ts (2)

6-6: Import statement for MaxVersionVector is correctly added.

This import is necessary for the new version vector-based garbage collection logic.

---

44-47: Garbage collection logic updated to use MaxVersionVector.

The update from MaxTimeTicket to MaxVersionVector is correctly implemented. This change should theoretically improve the precision of garbage collection by using version vectors.

Consider running performance benchmarks to ensure that the new garbage collection logic does not adversely affect performance.

packages/sdk/test/bench/text.bench.ts (3)

4-4: Import statement for MaxVersionVector is correctly added.

This import is necessary for the new version vector-based garbage collection logic.

---

52-55: Consistent update in garbage collection logic for benchmarkTextSplitGC.

The changes are consistent with those in benchmarkTextEditGC, correctly using MaxVersionVector for garbage collection. This should enhance the accuracy of garbage collection based on version vectors.

Ensure consistency in performance with the previous implementation by running benchmarks.

---

26-29: Garbage collection logic updated to use MaxVersionVector.

The update from MaxTimeTicket to MaxVersionVector is correctly implemented. This change should theoretically improve the precision of garbage collection by using version vectors.

Consider running performance benchmarks to ensure that the new garbage collection logic does not adversely affect performance.

Files that changed from the base of the PR and between 13f528b and 0c6a7db.

• examples/vanilla-codemirror6/src/main.ts (1 hunks)

examples/vanilla-codemirror6/src/main.ts (1)

31-42: Verify the impact of the document identifier change. The addition of event listeners is approved.

The event listeners for the beforeunload and unload events ensure proper cleanup of connections when the application is no longer active, improving resource management. This change is approved.

However, the TODO comment at line 31 indicates that the change to the document identifier is temporary.

Please verify if this change is intended to be permanent and assess its impact on the system. If it's a temporary change, consider reverting it or provide more context on when it will be reverted.

Run the following script to verify the usage of the document identifier:

Files that changed from the base of the PR and between f4f7ff4 and 40d5792.

• packages/sdk/buf.gen.yaml (1 hunks)
• packages/sdk/src/api/yorkie/v1/resources_pb.d.ts (4 hunks)
• packages/sdk/src/api/yorkie/v1/resources_pb.js (46 hunks)
• packages/sdk/src/api/yorkie/v1/yorkie_connect.d.ts (1 hunks)
• packages/sdk/src/api/yorkie/v1/yorkie_connect.js (1 hunks)
• packages/sdk/src/api/yorkie/v1/yorkie_pb.d.ts (1 hunks)
• packages/sdk/src/api/yorkie/v1/yorkie_pb.js (17 hunks)

• packages/sdk/src/api/yorkie/v1/yorkie_connect.d.ts
• packages/sdk/src/api/yorkie/v1/yorkie_connect.js
• packages/sdk/src/api/yorkie/v1/yorkie_pb.d.ts
• packages/sdk/src/api/yorkie/v1/yorkie_pb.js

packages/sdk/buf.gen.yaml (2)

11-11: LGTM. Ensure consistency across plugins and verify impact.

The change from target=ts to target=js+dts for the connect-es plugin aligns with the previous change for the es plugin, maintaining consistency in the code generation approach.

To ensure this change is applied consistently and doesn't cause any issues, please run the following verification:

#!/bin/bash
# Description: Verify consistency of plugin configurations and impact on connect-specific code

# Test: Check if all plugins use the same target
echo "Verifying consistency of plugin targets:"
rg --type yaml 'target=' buf.gen.yaml

# Test: Check for any connect-specific TypeScript files that might need updating
echo "Checking for connect-specific TypeScript files:"
fd -e ts . -p '*connect*' --exec echo "Found connect-related .ts file: {}"

# Test: Verify the presence of connect-specific .js and .d.ts files
echo "Verifying the presence of connect-specific .js and .d.ts files:"
fd -e js -e d.ts . -p '*connect*' --exec echo "Found connect-related file: {}"

This script will help ensure consistency across plugins and identify any connect-specific files that might be affected by this change.

---

6-6: LGTM. Verify impact on build process and consumers.

The change from target=ts to target=js+dts for the es plugin is appropriate. This will generate JavaScript files with separate TypeScript declaration files, which can improve compatibility and potentially optimize bundling.

To ensure this change doesn't negatively impact the project, please run the following verification:

This script will help identify any potential issues with the file generation or usage in the project.

packages/sdk/src/api/yorkie/v1/resources_pb.js (5)

Line range hint 26-47: Introduction of /*@__PURE__*/ Annotations

The /*@__PURE__*/ annotations have been added to exported constants and message types. These annotations help in tree-shaking and optimizing the code during minification. Verify that the build process respects these annotations and that they do not affect the runtime behavior.

Ensure that your bundler or minifier is configured to utilize the @__PURE__ annotations for dead code elimination.

---

86-91: Addition of Version Vector Fields to ChangePack

New fields snapshot_version_vector, min_synced_version_vector, and version_vector have been added to the ChangePack message. Ensure that these additions are correctly integrated into serialization and deserialization processes. Verify that all components interacting with ChangePack are updated to handle these new fields.

Run the following script to find all usages of ChangePack and check for handling of the new fields:

#!/bin/bash
# Description: Find usages of ChangePack and verify handling of new fields.

# Search for all instances where ChangePack is used.
rg --type js 'ChangePack' -A 10

# Check if new fields are accessed or set.
rg --type js 'snapshot_version_vector|min_synced_version_vector|version_vector'

---

111-120: Addition of version_vector Field to ChangeID

A new field version_vector has been added to the ChangeID message. Ensure that all instances where ChangeID is created or utilized are updated to handle this new field appropriately.

Run the following script to locate all usages of ChangeID and verify updates:

#!/bin/bash
# Description: Find usages of ChangeID and check for handling of version_vector.

# Search for all instances where ChangeID is used.
rg --type js 'ChangeID' -A 5

# Check if version_vector is being set or accessed.
rg --type js 'version_vector'

---

122-128: Introduction of VersionVector Message Type

A new message type VersionVector has been introduced. Confirm that this type is properly integrated and that any code interacting with version vectors is correctly implemented.

Run the following script to find all usages of VersionVector:

#!/bin/bash
# Description: Identify usages of VersionVector in the codebase.

# Search for all instances where VersionVector is used.
rg --type js 'VersionVector' -A 5

Ensure that the VersionVector is being used consistently and contributes to solving the garbage collection problem as intended.

---

16-16: Update of protoc-gen-es Version

The generator version has been updated to v1.10.0 from v1.6.0. Ensure that the new version does not introduce any breaking changes or compatibility issues with the existing codebase.

Run the following script to check for any deprecated or changed APIs that may affect the code:

packages/sdk/src/api/yorkie/v1/resources_pb.d.ts (3)

206-210: Confirm correct implementation of the versionVector field in ChangePack.

The versionVector field has been added to the ChangePack class. Verify that this field accurately represents document versions and is properly incorporated into the synchronization and conflict resolution processes.

Run the following script to check usage of versionVector:

#!/bin/bash
# Description: Search for references to `versionVector` in `ChangePack`.

# Test: Ensure consistent handling across the codebase.
rg 'ChangePack.*versionVector'

---

196-200: Verify integration of the new minSyncedVersionVector field.

A new field minSyncedVersionVector has been introduced in the ChangePack class. Ensure that this field is correctly utilized in synchronization logic and properly replaces the deprecated minSyncedTicket field where applicable.

Run the following script to locate usages of minSyncedVersionVector:

#!/bin/bash
# Description: Find all references to `minSyncedVersionVector`.

# Test: Ensure proper integration in synchronization mechanisms.
rg 'minSyncedVersionVector'

---

289-293: Ensure consistency of versionVector usage in ChangeID.

The versionVector field has been added to the ChangeID class. Confirm that all operations involving ChangeID are updated to account for this new field and that it integrates seamlessly with existing versioning logic.

Run the following script to find usages of ChangeID.versionVector:

#!/bin/bash
# Description: Locate references to `versionVector` within `ChangeID`.

# Test: Verify correct handling in all related operations.
rg 'ChangeID.*versionVector'

Files that changed from the base of the PR and between 40d5792 and 6f14089.

• packages/sdk/src/api/converter.ts (9 hunks)
• packages/sdk/src/api/yorkie/v1/resources.proto (2 hunks)
• packages/sdk/src/api/yorkie/v1/resources_pb.d.ts (5 hunks)
• packages/sdk/src/api/yorkie/v1/resources_pb.js (47 hunks)
• packages/sdk/src/document/change/change_pack.ts (6 hunks)
• packages/sdk/src/document/document.ts (12 hunks)
• packages/sdk/test/integration/gc_test.ts (16 hunks)

packages/sdk/src/document/change/change_pack.ts (5)

21-21: LGTM: Import statement for VersionVector is correctly added.

The import statement for VersionVector is properly placed and follows the project's import structure.

---

56-59: LGTM: versionVector property is correctly added.

The versionVector property is properly declared as private and optional. The accompanying comment provides a clear description of its purpose.

---

66-66: LGTM: Constructor and create method are correctly updated.

The changes to the constructor and create method are consistent and maintain the existing pattern of the class. The versionVector parameter is correctly added and handled in both methods.

Also applies to: 76-76, 86-86, 95-95

---

164-169: LGTM: getVersionVector method is correctly implemented.

The getVersionVector method is properly implemented as a public getter, following the existing pattern in the class. The return type and comment are appropriate.

---

56-59: Please provide more context on how these changes address the GC problem.

The introduction of versionVector to the ChangePack class seems to align with the PR objective of addressing a garbage collection (GC) problem. However, it's not immediately clear how these changes specifically contribute to solving the GC issue.

Could you please provide more context on:

1. How the versionVector is used in the GC process?
2. Are there any other files or components that interact with this new versionVector property to facilitate GC?
3. Are there any performance implications or benefits expected from this change?

This additional information would help in understanding the full scope and impact of these changes.

To help verify the usage and impact of the versionVector, we can run the following script:

Also applies to: 66-66, 76-76, 86-86, 95-95, 164-169

packages/sdk/src/api/yorkie/v1/resources_pb.js (5)

120-128: New VersionVector message type added

The addition of the VersionVector message type aligns with the PR objective of introducing a version vector to solve a GC problem. This new type will help track the version state across different actors in the system.

---

Line range hint 80-89: ChangePack updated to include version_vector

The ChangePack message type has been updated to include a version_vector field. This change is consistent with the introduction of the version vector concept and should help in tracking the state of changes across the system.

---

109-118: ChangeID updated to include version_vector

The ChangeID message type has been updated to include a version_vector field. This change is consistent with the introduction of the version vector concept and should help in uniquely identifying changes across the system.

---

Line range hint 26-44: Consistent use of /*@__PURE__*/ annotation

The changes consistently add the /*@__PURE__*/ annotation to all proto3.makeEnum and proto3.makeMessageType calls throughout the file. This annotation is used by some JavaScript minifiers and bundlers to perform dead code elimination more effectively.

These changes are likely a result of the upgrade to protoc-gen-es v1.10.0 and should improve the efficiency of the generated code when used with certain build tools.

Also applies to: 49-57, 66-72, 96-104, 133-149, 152-160, 166-174, 180-188, 194-201, 207-220, 229-237, 243-254, 259-266, 272-284, 289-302, 306-314, 320-330, 335-343, 349-357, 363-374, 378-386, 392-403, 407-415, 421-427, 432-438, 443-450, 455-464, 469-475, 480-492, 497-502, 507-513, 518-524, 529-536, 541-554, 559-568, 572-578, 583-593, 598-604, 609-617, 622-627, 632-638, 643-650, 655-662, 667-673, 678-684

---

Line range hint 1-684: Summary of changes and their impact

The changes in this file are primarily due to the upgrade of protoc-gen-es from v1.6.0 to v1.10.0. The main impacts are:

1. Consistent addition of /*@__PURE__*/ annotations, which may improve dead code elimination in some build processes.
2. Introduction of a new VersionVector message type.
3. Integration of version_vector fields into ChangePack and ChangeID message types.

These changes, especially the introduction of the version vector concept, align well with the PR objective of solving a GC problem. The changes appear to be well-structured and should improve the system's ability to track and manage document states across different actors.

packages/sdk/src/api/yorkie/v1/resources.proto (1)

Line range hint 47-68: Ensure backward compatibility with existing clients

While adding new fields is generally backward compatible in Protobuf, it's important to verify that existing clients and servers can handle these changes without issues. Old clients might ignore the new fields, but ensure that the deprecation of min_synced_ticket and addition of version_vector do not introduce unintended side effects.

Run the following script to identify potentially impacted areas and ensure compatibility:

✅ Verification successful

Backward compatibility verified: min_synced_ticket is only defined in resources.proto

The deprecated field min_synced_ticket is not used elsewhere in the codebase, ensuring that its removal does not impact existing clients.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find usages of the affected fields and messages to assess impact.

# Test: Search for usages of 'min_synced_ticket' in the codebase.
rg --type proto --type go --type ts 'min_synced_ticket'

# Test: Identify all serialization and deserialization code involving 'ChangePack' and 'ChangeID'.
rg --type go --type ts 'ChangePack|ChangeID'

# Note: Review the results to ensure that the handling of new and deprecated fields is appropriate.

Length of output: 18004

---

Script:

#!/bin/bash
# Description: Correctly search for usages of 'min_synced_ticket' within proto files.

# Find all .proto files and search for 'min_synced_ticket'
fd --extension proto | xargs rg 'min_synced_ticket'

Length of output: 148

packages/sdk/src/api/converter.ts (2)

1523-1527: Handle Possible Undefined Value from 'toVersionVector'

In the function versionVectorToHex, there's an assumption that toVersionVector(vector) will not return undefined due to the non-null assertion operator !:

const pbVersionVector = toVersionVector(vector)!;

If vector can be undefined, this could lead to a runtime error.

Ensure that vector is indeed defined before calling toVersionVector, or modify the function to handle undefined values gracefully. You might consider adding a check:

if (!vector) {
  return '';
}
const pbVersionVector = toVersionVector(vector);

Or, if toVersionVector can handle undefined:

-const pbVersionVector = toVersionVector(vector)!;
+const pbVersionVector = toVersionVector(vector);

And adjust the subsequent code to handle the possibility of pbVersionVector being undefined.

---

803-803: Ensure Exported Functions are Consistently Used

The functions versionVectorToHex and hexToVersionVector are newly introduced and exported at line 1671-1672:

versionVectorToHex,
hexToVersionVector,

However, their usage in the codebase appears limited.

Run the following script to verify where these functions are used:

This will help determine if these functions are utilized appropriately or if additional integration is needed.

Also applies to: 1671-1672

packages/sdk/test/integration/gc_test.ts (1)

7-8: Imports are correctly added.

The new imports of MaxVersionVector, versionVectorHelper, and Long are appropriate and necessary for the updated garbage collection logic and version vector handling.

packages/sdk/src/api/yorkie/v1/resources_pb.d.ts (2)

16-16: Update to protoc-gen-es v1.10.0 is acceptable

The update to the code generator version ensures compatibility with the latest Protocol Buffers features.

---

279-283: Verify serialization and deserialization of the new versionVector field in ChangeID

Ensure that the versionVector field added to ChangeID is properly integrated into all relevant serialization and deserialization methods.

Run the following script to verify:

#!/bin/bash
# Description: Verify that 'versionVector' is properly serialized and deserialized in 'ChangeID'.

# Search for serialization methods handling 'versionVector' in 'ChangeID'
rg 'serialize.*versionVector' packages/sdk/src

# Search for deserialization methods handling 'versionVector' in 'ChangeID'
rg 'deserialize.*versionVector' packages/sdk/src

Files that changed from the base of the PR and between 6f14089 and 3aff0b5.

• .eslintignore (1 hunks)
• CHANGELOG.md (1 hunks)
• examples/react-tldraw/src/tldraw.d.ts (1 hunks)
• examples/vanilla-quill/src/main.ts (1 hunks)
• examples/vanilla-quill/src/type.ts (1 hunks)
• package.json (1 hunks)
• packages/sdk/buf.gen.yaml (1 hunks)
• packages/sdk/package.json (1 hunks)
• packages/sdk/src/api/converter.ts (12 hunks)
• packages/sdk/src/api/yorkie/v1/resources.proto (6 hunks)
• packages/sdk/src/api/yorkie/v1/resources_pb.d.ts (4 hunks)
• packages/sdk/src/api/yorkie/v1/resources_pb.js (0 hunks)
• packages/sdk/src/api/yorkie/v1/resources_pb.ts (1 hunks)
• packages/sdk/src/api/yorkie/v1/yorkie.proto (1 hunks)
• packages/sdk/src/api/yorkie/v1/yorkie_connect.ts (2 hunks)
• packages/sdk/src/api/yorkie/v1/yorkie_pb.js (0 hunks)
• packages/sdk/src/api/yorkie/v1/yorkie_pb.ts (1 hunks)
• packages/sdk/src/client/client.ts (5 hunks)
• packages/sdk/src/devtools/types.ts (1 hunks)
• packages/sdk/src/document/change/change_id.ts (6 hunks)
• packages/sdk/src/document/change/checkpoint.ts (5 hunks)
• packages/sdk/src/document/change/context.ts (1 hunks)
• packages/sdk/src/document/document.ts (14 hunks)
• packages/sdk/src/document/json/tree.ts (2 hunks)
• packages/sdk/src/document/time/ticket.ts (6 hunks)
• packages/sdk/src/document/time/version_vector.ts (1 hunks)
• packages/sdk/src/util/object.ts (3 hunks)
• packages/sdk/src/util/splay_tree.ts (2 hunks)
• packages/sdk/src/util/validator.ts (1 hunks)
• packages/sdk/test/bench/splay_tree.bench.ts (1 hunks)
• packages/sdk/test/integration/client_test.ts (11 hunks)
• packages/sdk/test/integration/integration_helper.ts (2 hunks)
• packages/sdk/test/integration/object_test.ts (10 hunks)
• packages/sdk/tsconfig.json (1 hunks)

• packages/sdk/src/api/yorkie/v1/resources_pb.js
• packages/sdk/src/api/yorkie/v1/yorkie_pb.js

• .eslintignore
• package.json
• packages/sdk/package.json
• packages/sdk/src/api/yorkie/v1/yorkie.proto
• packages/sdk/src/api/yorkie/v1/yorkie_connect.ts

packages/sdk/buf.gen.yaml (1)

7-7: LGTM! Verify generated code imports.

The changes from js_import_style=module to import_extension=none for both es and connect-es plugins are appropriate. This modification aligns with modern TypeScript practices and can improve compatibility across different module systems.

To ensure these changes have the intended effect, please verify the generated code:

Ensure that the import statements in the generated code do not include file extensions.

Also applies to: 12-12

examples/react-tldraw/src/tldraw.d.ts (1)

1-6: LGTM! The interface extension aligns with the PR objective.

The extension of the TDUser interface with Indexable is a good approach to introduce the version vector functionality to the existing type from the external library. This change allows TDUser to inherit properties and methods from Indexable, which is likely crucial for implementing the version vector feature mentioned in the PR objectives.

packages/sdk/src/util/validator.ts (1)

Line range hint 20-30: Clarify the relevance of this change to the PR objectives

While the type safety improvement in validateSerializable is valuable, it's not immediately clear how this change relates to the PR's main objective of "Introducing version vector to solve GC problem".

Could you please clarify:

1. How does this function relate to the version vector implementation or garbage collection improvements?
2. Is this part of a broader effort to enhance type safety across the codebase?
3. Are there any specific use cases where this function is critical for the version vector or GC functionality?

To help understand the context, let's check for usage of this function:

This will help us understand where and how this function is used in the context of version vectors or garbage collection.

packages/sdk/src/util/object.ts (1)

17-17: LGTM: Import statement for Indexable type

The import of Indexable from the document module is appropriate and necessary for the subsequent type changes.

packages/sdk/test/integration/integration_helper.ts (2)

16-19: Improved type flexibility with new generic parameter.

The addition of the generic type parameter P extends Indexable = Indexable enhances the function's flexibility, allowing it to work with more specific document types while maintaining backward compatibility. This change aligns well with the introduction of the VersionVector class mentioned in the PR summary.

---

Line range hint 48-48: Consistent update to assertUndoRedo function.

The assertUndoRedo function has been correctly updated to use the new generic type P extends Indexable. This change maintains consistency with the updates made to withTwoClientsAndDocuments.

Overall, these changes improve the type flexibility of the integration tests, allowing for more specific document types to be used. This aligns well with the introduction of the VersionVector class and enhances the SDK's ability to handle various document structures in testing scenarios.

packages/sdk/src/devtools/types.ts (2)

20-20: Approve: Centralized Json type import improves consistency

The import of the Json type from a central location (@yorkie-js-sdk/src/document/document) is a positive change. This centralization:

1. Enhances consistency across the codebase
2. Reduces duplication of type definitions
3. Simplifies future updates to the Json type

The usage of this imported type in the Client interface maintains type safety while potentially allowing for more flexible presence data.

---

Line range hint 1-91: Summary: Minimal, focused change improves type consistency

The modification to this file is part of a broader effort to enhance type safety and consistency across the SDK. By centralizing the Json type definition, the change:

1. Maintains the overall structure and functionality of the types defined in this file
2. Aligns with updates in other parts of the SDK, such as the Client class and Document class
3. Sets the stage for more consistent handling of JSON data throughout the codebase

This focused change demonstrates a thoughtful approach to refactoring, minimizing the risk of introducing errors while improving the overall design of the SDK.

packages/sdk/src/document/change/checkpoint.ts (5)

23-23: Approved: Transition to native bigint type

The change from Long to bigint for serverSeq is a good improvement. It leverages native JavaScript capabilities for handling large integers, which can potentially improve performance and reduce dependencies on external libraries.

---

26-26: Approved: Constructor signature updated for consistency

The constructor signature has been correctly updated to use bigint for the serverSeq parameter, maintaining consistency with the property type change.

---

34-34: Approved: Static method signature updated for consistency

The static of method signature has been correctly updated to use bigint for the serverSeq parameter, maintaining consistency with the property and constructor changes.

---

58-59: Approved: Simplified comparison logic

The change from using the greaterThan method to a direct > operator comparison is appropriate for bigint values. This change improves readability while maintaining the same logic of selecting the larger serverSeq value.

---

82-82: Approved: Return type updated for consistency

The getServerSeq method's return type has been correctly updated to bigint, maintaining consistency with the property type change.

packages/sdk/src/document/change/context.ts (1)

162-162: Improved type safety in getReversePresence method

The addition of the as keyof P type assertion enhances type safety when accessing properties of this.previousPresence. This change ensures that TypeScript correctly understands that key is a valid key of the generic type P, which extends Indexable.

This modification aligns with TypeScript best practices for working with dynamic keys in generic types, providing better type checking and potentially catching errors at compile-time rather than runtime.

examples/vanilla-quill/src/main.ts (1)

32-32: Verify the impact of directly returning the embed attribute.

The change from parsing the embed attribute as JSON to returning it directly could have implications on how embedded content is processed throughout the system.

Please confirm:

1. Is this change intentional?
2. Are there any other parts of the system that expect the embed value to be a parsed object?
3. Could this lead to inconsistencies if some embed values are stored as strings and others as objects?

To help verify the usage of embed across the codebase, you can run the following script:

This will help identify other locations where the embed attribute is used and potentially affected by this change.

✅ Verification successful

Verified the impact of directly returning the embed attribute.

No issues found with directly returning the embed attribute in examples/vanilla-quill/src/main.ts.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage of 'embed' attribute in the codebase
rg -i 'embed.*attribute' --type ts

Length of output: 231

packages/sdk/src/document/json/tree.ts (2)

85-92: Improved attribute handling in buildDescendants

The changes enhance type safety and consistency in attribute handling:

1. Using const for attributes improves immutability.
2. Introducing stringifiedAttributes ensures consistent stringification of attribute values.
3. Iterating over stringifiedAttributes in the loop guarantees that all values are properly stringified when set in the RHT structure.

These modifications make the code more robust and less prone to type-related errors.

---

124-131: Consistent improvement in attribute handling in createCRDTTreeNode

The changes in this function mirror those in buildDescendants:

1. attributes is now a constant, improving immutability.
2. stringifiedAttributes is introduced to ensure consistent stringification.
3. The loop iterates over stringifiedAttributes, guaranteeing properly stringified values.

This consistency in approach across functions enhances code maintainability and reduces the likelihood of type-related errors.

CHANGELOG.md (1)

Line range hint 1-1024: CHANGELOG.md effectively documents Yorkie JS SDK history.

The CHANGELOG.md file is comprehensive, well-structured, and provides a clear history of changes for the Yorkie JS SDK. It follows the Keep a Changelog format and adheres to Semantic Versioning, making it easy for users and developers to understand the project's evolution.

The most recent changes (version 0.5.1) are properly documented with clear categorization. The consistent use of GitHub usernames and pull request links throughout the document is helpful for traceability.

The suggested improvements (URL formatting and potential recategorization of the linting automation entry) are minor and do not detract from the overall quality and usefulness of the changelog.

🧰 Tools

🪛 Markdownlint

14-14: null
Bare URL used

(MD034, no-bare-urls)

---

18-18: null
Bare URL used

(MD034, no-bare-urls)

---

19-19: null
Bare URL used

(MD034, no-bare-urls)

---

23-23: null
Bare URL used

(MD034, no-bare-urls)

packages/sdk/src/document/time/ticket.ts (2)

48-52: Ensure 'bigint' is supported in all target environments

The TimeTicket class now uses bigint for the lamport property. Since bigint was introduced in ECMAScript 2020, it may not be supported in all environments, such as older browsers or Node.js versions prior to 10.4.0. Please verify that all intended environments for this SDK support bigint, or consider adding necessary polyfills or transpilation steps.

---

163-167: Comparison logic correctly updated for 'bigint'

The compare method now uses standard comparison operators with bigint, ensuring accurate comparisons between TimeTicket instances.

packages/sdk/src/api/yorkie/v1/resources.proto (7)

49-49: Addition of 'version_vector' to 'ChangePack' is appropriate

The inclusion of the version_vector field in the ChangePack message enhances version tracking for changes. The field is correctly defined and numbered.

---

61-62: Verify type changes for 'server_seq' and 'lamport' in 'ChangeID'

Changing server_seq and lamport to int64 in the ChangeID message ensures consistency with other parts of the code. Please verify that this type change doesn't affect backward compatibility or data integrity with existing clients and stored data.

---

64-64: Inclusion of 'version_vector' in 'ChangeID' enhances synchronization

Adding the version_vector field to the ChangeID message provides additional context for change tracking and synchronization. Ensure that all serialization and deserialization processes accommodate this new field.

---

67-68: Definition of 'VersionVector' message is correct

The VersionVector message is properly defined with a map of string to int64. This structure effectively captures the version information needed for synchronization.

---

145-150: Ensure comprehensive support for the new 'ArraySet' operation

The introduction of the ArraySet operation extends the capabilities of array manipulations. Please verify that:

• Serialization and deserialization logic correctly handle ArraySet.
• All relevant client and server logic is updated to process this new operation.
• Documentation and tests are added or updated to reflect this addition.

---

163-163: Update operation handling to include 'array_set'

With array_set added to the oneof body in the Operation message, ensure that:

• All switch statements or conditional logic processing Operation messages are updated to handle the array_set case.
• Appropriate error handling is in place for unsupported operation types.

---

341-341: Confirm type changes for 'server_seq' and 'lamport' in 'Checkpoint' and 'TimeTicket'

Changing server_seq and lamport to int64 promotes consistency across the codebase. Please ensure that:

• There are no issues with data overflows or sign mismatches.
• Existing data and clients remain compatible, or necessary migration steps are taken.

Also applies to: 352-352

packages/sdk/src/util/splay_tree.ts (2)

214-214: Verify the impact of splaying in the find method

By adding this.splayNode(node); in the find method, each call to find will splay the found node to the root of the tree. While this conforms to the splay tree properties—bringing frequently accessed nodes closer to the root for faster access—this change may affect performance characteristics and the overall tree structure during traversal operations. Please verify that this modification aligns with the intended behavior of the tree in your application context.

---

229-230: Ensure correct index calculation in indexOf after splaying

The updated indexOf method now splays the target node and returns this.root!.getLeftWeight();. Since splaying alters the tree structure by moving the node to the root, confirm that this approach accurately computes the node's original index as expected by other components of the system. This change simplifies the method, but it's crucial to ensure that it doesn't introduce inconsistencies in index calculations elsewhere in the application.

packages/sdk/src/client/client.ts (3)

621-622: Enhance Type Safety with Json Type

Changing the payload type from any to Json improves type safety, ensuring that only serializable JSON objects are accepted. This is a good practice that enhances code reliability.

---

690-691: Unreachable Code Due to Promise Handling

The throw err; statements inside the .catch block may not propagate the error as intended in asynchronous code. Adjusting to return Promise.reject(err); ensures the promise chain correctly handles the error.

---

319-319: ⚠️ Potential issue

Fix Typo in Variable Name unsubscribeBroacastEvent

The variable unsubscribeBroacastEvent has a typo. It should be unsubscribeBroadcastEvent.

Apply this diff to correct the variable name:

-const unsubscribeBroacastEvent = doc.subscribe(
+const unsubscribeBroadcastEvent = doc.subscribe(
  'local-broadcast',

Likely invalid or redundant comment.

packages/sdk/src/api/yorkie/v1/yorkie_pb.ts (2)

1-15: Approved: License header and autogenerated metadata are correctly included

The Apache license header and autogenerated comments are appropriately included at the top of the file.

---

21-738: Approved: Generated classes and methods are consistent with proto definitions

The generated classes and methods appear correct and align with the .proto definitions. Serialization and deserialization methods are properly implemented, ensuring accurate message handling.

packages/sdk/test/integration/object_test.ts (1)

659-659: Correct use of BigInt literals in server sequence number assertions

The assertions now use BigInt literals (e.g., 1n, 2n, 4n, 5n) when comparing server sequence numbers, which is appropriate for accurate comparisons with large integers.

Also applies to: 670-670, 686-686, 701-701, 712-712, 735-735, 746-746, 762-762, 777-777, 788-788

packages/sdk/src/api/converter.ts (6)

153-154: Code Change Looks Good

The changes in the toCheckpoint function correctly use checkpoint.getServerSeq() and checkpoint.getClientSeq(). The update ensures that the server and client sequences are accurately converted to the Protobuf format.

---

164-166: Updates to toChangeID Function Are Correct

The addition of lamport: changeID.getLamport() and versionVector: toVersionVector(changeID.getVersionVector()) in the toChangeID function appropriately includes the lamport timestamp and version vector in the conversion to Protobuf format. This aligns with the enhancements made to handle version vectors.

---

179-181: Code Change Looks Good

The updates in toTimeTicket function properly utilize ticket.getLamport(), ticket.getDelimiter(), and toUint8Array(ticket.getActorID()) for converting a TimeTicket to its Protobuf representation. This ensures accurate serialization of time tickets.

---

1351-1351: Conversion to BigInt in fromCheckpoint Function Is Correct

In fromCheckpoint, converting pbCheckpoint.serverSeq to BigInt is necessary for consistency with the Checkpoint model, which expects a bigint for the server sequence. This change ensures accurate representation of large numerical values.

---

1365-1365: Inclusion of fromVersionVector Is Appropriate

The addition of fromVersionVector(pbPack.versionVector) in the ChangePack.create method correctly integrates the version vector from the Protobuf format to the model format, enhancing the change pack's capability to track version vectors.

---

1663-1664: Exporting versionVectorToHex and hexToVersionVector Functions

Adding versionVectorToHex and hexToVersionVector to the exported converter object makes these utility functions available for external use, facilitating the conversion between VersionVector instances and their hexadecimal representations.

packages/sdk/src/document/document.ts (7)

81-96: BroadcastOptions Interface Definition Looks Good

The BroadcastOptions interface is well-defined with appropriate optional properties for error handling and retry mechanisms. The documentation comments are clear and consistent.

---

324-328: Addition of snapshotVector Enhances Snapshot Management

The inclusion of snapshotVector in the SnapshotEvent interface provides additional context for snapshot synchronization. The types and documentation are consistent, and this change aligns with the updates made in related methods.

---

444-453: Update to Indexable Improves Type Safety

Changing the Indexable type from Record<string, any> to Record<string, Json> enhances type safety and aligns with the defined Json types. The Json type definitions correctly represent JSON data structures.

Also applies to: 459-459

---

Line range hint 1404-1426: applySnapshot Method Updated Appropriately

The applySnapshot method now includes snapshotVector as a parameter, which enhances the synchronization process by incorporating version vectors. The method's logic correctly updates the changeID with setClocks, and the inclusion of snapshotVector in the published event ensures that subscribers receive the necessary context.

---

2124-2133: broadcast Method Enhancements

The broadcast method now accepts options?: BroadcastOptions, allowing for better control over error handling and retry logic when broadcasting events. The changes are logically sound and integrate well with the existing event system.

---

2134-2139: Addition of getVersionVector Method

The new getVersionVector method provides a straightforward way to retrieve the document's version vector, which is essential for synchronization and conflict resolution. The implementation correctly delegates to this.changeID.getVersionVector().

---

1892-1900: Implementation of filterVersionVector Method

The filterVersionVector method effectively filters out detached clients' lamport clocks from the version vector, which is crucial for maintaining accurate synchronization states. Ensure that the VersionVector.filter method is implemented correctly and handles edge cases where version vectors might be empty or contain unexpected values.

Run the following script to verify the existence and proper implementation of the filter method in VersionVector:

packages/sdk/src/api/yorkie/v1/resources_pb.ts (3)

2721-2761: Verify Usage of DocEvent and DocEventBody

The classes DocEvent and DocEventBody are defined but it's unclear how they're utilized. Confirm that these classes are integrated into the event handling system as intended.

---

227-230: ⚠️ Potential issue

Deprecated Field min_synced_ticket in ChangePack

The min_synced_ticket field in the ChangePack class is marked as deprecated. Ensure that it's not being used elsewhere in the codebase and consider removing it to prevent confusion.

Please confirm if there are any remaining references:

✅ Verification successful

Deprecated Field min_synced_ticket Verified for Removal

All references to min_synced_ticket are confined to its definition in the protobuf files. There are no usages elsewhere in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for references to the deprecated `min_synced_ticket` field.

# Search for usage of `min_synced_ticket`.
rg 'min_synced_ticket'

Length of output: 480

---

355-357: Ensure Proper Initialization of versionVector in ChangeID

The VersionVector field has been added to the ChangeID class. Verify that all instances of ChangeID are updated to initialize this new field to avoid undefined values that could lead to runtime errors.

Consider running the following script to identify uninitialized versionVector fields:

Files that changed from the base of the PR and between 3aff0b5 and 7e39737.

• packages/sdk/src/document/document.ts (15 hunks)
• packages/sdk/test/helper/helper.ts (3 hunks)
• packages/sdk/test/integration/gc_test.ts (16 hunks)

packages/sdk/test/helper/helper.ts (4)

31-31: LGTM!

The import of MaxLamport and TimeTicket is appropriate and supports the new functionality.

---

41-42: LGTM!

Imports for InitialActorID and VersionVector are correctly added.

---

293-305: LGTM!

The MaxVersionVector function correctly initializes a VersionVector with the provided actors and MaxLamport values.

---

307-328: LGTM!

The versionVectorHelper function effectively validates the VersionVector against the provided actor data.

packages/sdk/test/integration/gc_test.ts (1)

1250-1250: ⚠️ Potential issue

Avoid using the delete operator on properties

For better performance, assign undefined to object properties instead of deleting them.

Apply this diff:

-          delete root['2'];
+          root['2'] = undefined;

Likely invalid or redundant comment.